-
Notifications
You must be signed in to change notification settings - Fork 5.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support placement_group=None in PlacementGroupSchedulingStrategy #27370
Conversation
Signed-off-by: Jiajun Yao <[email protected]>
Signed-off-by: Jiajun Yao <[email protected]>
because the resource request {'CPU': 10} cannot fit into any bundles for the placement group, [{'CPU': 1.0}]. | ||
|
||
**Expected behavior**: The above executes. | ||
|
||
**Fix**: In the ``@ray.remote`` declaration of tasks | ||
called by ``create_task_that_uses_resources()`` , include a | ||
``placement_group=None``. | ||
``scheduling_strategy=PlacementGroupSchedulingStrategy(placement_group=None)``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still feel a bit weird that we specify a strategy just to not use that strategy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We allow both right? Because in Datasets, we override using DEFAULT.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, currently we allow both.
…-project#27370) We decided to allow escaping the parent pg via `PlacementGroupSchedulingStrategy(placement_group=None)` instead of using "DEFAULT". Our doc is updated with that but in the code it's still not allowed. Signed-off-by: Stefan van der Kleij <[email protected]>
Why are these changes needed?
We decided to allow escaping the parent pg via
PlacementGroupSchedulingStrategy(placement_group=None)
instead of using "DEFAULT". Our doc is updated with that but in the code it's still not allowed.Related issue number
Closes #27328
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.